Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties #35108

Closed
wants to merge 1 commit into from

Conversation

cortinico
Copy link
Contributor

Summary:
I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the gradle.properties file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Differential Revision: D40762872

@cortinico cortinico requested a review from hramos as a code owner October 27, 2022 15:53
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Oct 27, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40762872

@react-native-bot react-native-bot added the Platform: Android Android applications. label Oct 27, 2022
@@ -38,3 +38,7 @@ reactNativeArchitectures=armeabi-v7a,arm64-v8a,x86,x86_64
# to write custom TurboModules/Fabric components OR use libraries that
# are providing them.
newArchEnabled=false

# Use this property to enable or disable the Hermes JS engine.
# If set to true, you will be using JSC instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo

cortinico added a commit to cortinico/react-native that referenced this pull request Oct 27, 2022
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: ccb50740f72bf9be17e1dd241a81bd7da42a7708
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40762872

cortinico added a commit to cortinico/react-native that referenced this pull request Oct 27, 2022
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: 9373a65e7d44840dafe4d7f333cf83b66d146eea
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40762872

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40762872

cortinico added a commit to cortinico/react-native that referenced this pull request Oct 27, 2022
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: 289f04f87b278097337db88a4474beecb2f2717a
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: 58fd2c87b3460da21f2560d7beb93710b626cda5
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40762872

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,001,888 +495
android hermes armeabi-v7a 6,378,328 +461
android hermes x86 7,414,780 +832
android hermes x86_64 7,278,674 +802
android jsc arm64-v8a 8,867,565 +0
android jsc armeabi-v7a 7,605,882 +0
android jsc x86 8,925,465 +0
android jsc x86_64 9,408,701 +0

Base commit: ba5454c
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ba5454c
Branch: main

@pull-bot
Copy link

PR build artifact for be72feb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@pull-bot
Copy link

PR build artifact for be72feb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @cortinico in 6a43faf.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 27, 2022
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…le.properties (facebook#35108)

Summary:
Pull Request resolved: facebook#35108

I've rewritten the comment in the android/app/build.gradle.
They were really old, contained wrong links and most of the people ignored it.

I've also moved the enabling of Hermes to the `gradle.properties` file.
RNGP still supports the old method, but we must be sure that we notify
library authors if they were reading project.ext.react.enableHermes in the past
(also the website needs to be updated).

I've also cleaned up the CircleCI setup as now we can specify Hermes enabled/disabled
via the CLI (this will also make easier to do e2e testing).

Changelog:
[Android] [Changed] - Cleanup the template documentation after RNGP & hermesEnabled to gradle.properties

Reviewed By: cipolleschi

Differential Revision: D40762872

fbshipit-source-id: 2c09245e0a923faac53cc6c8a89e99788ae47f8a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants